fix: improve terminal output capture for first command in new terminals #9020
+43
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add 200ms delay for first command in newly created terminals to ensure shell integration is ready - Improve stream data handling to recover from missing shell integration markers - Track whether terminal is newly created and if first command has been executed - Add fallback logic to treat accumulated data as output if markers aren't found after 500 chars This fixes the race condition where the first command's output might not be captured properly in freshly created terminals, especially with chained commands. Fixes #9019
- Fix logic error where streamDataReceived check was always false - Replace with chunksReceived counter to properly track stream chunks - Add detailed documentation explaining timing values and rationale - Improve comments to explain the 200ms delay and 500 char threshold choices
dosubot
bot
added
the
size:M
|
Review complete. The implementation correctly addresses the terminal output capture race condition with a 200ms delay for first commands in new terminals and improved fallback logic. Patch 2 fixes the logic error from patch 1 by replacing the always-false boolean check with a chunk counter. No issues found. Mention @roomote in a comment to trigger your PR Fixer agent and make changes to this pull request. |
dosubot
bot
added
bug
hannesrudolph
added
the
Issue/PR - Triage
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR attempts to address Issue #9019 where terminal command output is not reliably captured when it's the first command in a freshly created terminal session. The issue is particularly noticeable with chained commands like
ENV=hello && echo $ENV.Problem
There's a race condition where the shell integration stream might not be fully ready when the first command is executed in a newly created terminal, causing the output to be lost even though the command executes successfully.
Solution
This fix implements a two-pronged approach:
Changes
isNewlyCreatedflag)streamDataReceivedwithchunksReceivedcounter)Testing
ENV=hello && echo $ENVnow reliably captures outputNotes
Fixes #9019
Feedback and guidance are welcome!
Important
Fixes terminal output capture race condition by adding delay for first command and improving stream handling in new terminals.
Terminal.tsto ensure shell integration readiness.TerminalProcess.tsto manage delayed or missing shell integration markers.isNewlyCreatedandfirstCommandExecutedflags inTerminalclass.streamDataReceivedwithchunksReceivedcounter inTerminalProcess.TerminalProcessto treat data as output if markers aren't found after 500 characters.ENV=hello && echo $ENVcase.This description was created by
for 087ddf2. You can customize this summary. It will automatically update as commits are pushed.